Claimed resources synchronization by lhaendler · Pull Request #155 · kbind-dev/kbind

lhaendler · 2023-03-09T11:47:24Z

Discussion doc: https://docs.google.com/document/d/1Mfk2wLIURqAwSjoTimuZlL83BhslTiqV/edit#

Scope

The prototype only implements one basic use case: downsync of all resources of a type.

This includes association of the resources to the api export in the backend, synchronization by the konnector, and basic prompts for the claim in the bind cli

Current limitations

No filtering implemented
No support for adoption
No support for donation
No support for overrides, excludings, fields, alwaysRecreate

sttts · 2023-03-09T15:04:25Z

+	ClaimRejected AcceptablePermissionClaimState = "Rejected"
+)
+
+// PermissionClaim identifies an object by GR and identity hash.


Suggested change

// PermissionClaim identifies an object by GR and identity hash.

// PermissionClaim selects objects of a GVR that the service provider ...

sttts · 2023-03-09T15:05:08Z

+type PermissionClaim struct {
+	GroupResource `json:","`
+
+	Version string `json:"version"`


valdiation: required and non-empty

can't we simply reference GroupVersionResource, why is GroupResource and Version separate?

non-empty need validation, aka minlength

sttts · 2023-03-09T15:05:42Z

+	Version string `json:"version"`
+
+	// selector selects which resources are affected by this claim.
+	Selector ResourceSelector `json:"selector,omitempty"`


omitempty has no effect on structs. We could turn it into a pointer if that's prettier.

sttts · 2023-03-09T15:05:58Z

+	Selector ResourceSelector `json:"selector,omitempty"`
+
+	// required indicates whether the APIServiceBinding will work if this claim is not accepted.
+	Required bool `json:"required"`


default true?

I think it's better to make it opt in. It would make people think about whether it is actually needed. Since required claims cannot be denied, we should guide users to use required as little as needed by making the default false.

Since required claims cannot be denied

we should make this fact part of the API doc.

sttts · 2023-03-09T15:06:32Z

+	// Global claims global resources for the given group/resource.
+	// This is mutually exclusive with resourceSelector.
+	// +optional
+	Global bool `json:"global,omitempty"`


is this All?

I mostly added it to support cluster-scoped resources. However I'm not sure if that's actually something we want to enable. The problem being that given any claim but single-name claims, the user could have access to cluster-scoped objects of other bind consumers.

I am not convinced we should not allow cluster scoped resources. If we do, we lift the concept of a namespace to be a tenancy boundary which might actually not be desired. If used i.e. with kcp, claiming cluster-scoped resources is actually fine, as tenancy boundaries are defined one level up, at the workspace level.

sttts · 2023-03-09T15:07:04Z

+	//
+	// create determines whether the kube-bind konnector will sync matching objects from the
+	// provider side down to the consumer cluster.
+	Create CreateOptions `json:"create"`


this must be a pointer. There is no need that a service provider claims creation.

sttts · 2023-03-09T15:07:17Z

+
+	// adopt set to true means that objects created by the consumer are adopted by the provider.
+	// i.e. the provider will become the owner.
+	Adopt bool `json:"adopt"`


sttts · 2023-03-09T15:07:33Z

+
+	// update lists a number of claimed permissions for the provider.
+	// "field" and "preserving" are mutually exclusive.
+	Update UpdateOptions `json:"update"`


must be a pointer, omitempty

sttts · 2023-03-09T15:08:24Z

+	// providerOverrides will make the provider override any object that might already exist
+	// in the consumer cluster if it has the same namespaced name as a resource created by the
+	// provider.


Suggested change

// providerOverrides will make the provider override any object that might already exist

// in the consumer cluster if it has the same namespaced name as a resource created by the

// provider.

// providerOverrides will make the provider override any object that might already exist

// in the consumer cluster if it has the same namespaced name as a resource created by the

// provider, but is not the result of syncing.

sttts · 2023-03-09T15:08:33Z

+type CreateOptions struct {
+	// donate set to true means that a newly created object by the provider is immediately owned by hte consumer.
+	// If false, the object stays in ownership of the provider
+	Donate bool `json:"donate"`


sttts · 2023-03-09T15:10:02Z

+	// only for owner provider
+	// When recreateWhenConsumerSideDeleted is true the provider will recreate the object
+	// in case the object is missing on the consumer side. Even if the consumer mistakenly or intentionally


Suggested change

// only for owner provider

// When recreateWhenConsumerSideDeleted is true the provider will recreate the object

// in case the object is missing on the consumer side. Even if the consumer mistakenly or intentionally

// recreateWhenConsumerSideDeleted set to true means the provider will recreate the object

// in case the object is missing on the consumer side. Even if the consumer mistakenly or intentionally.

sttts · 2023-03-09T15:31:31Z

+	// fields are the fields owned by the owner of the claim. If the owner sets values of those
+	// fields, they will be synced to the other participant.
+	// Mutually exclusive with preservings.


would not use the word "owner" here. The claim owner is always the provider, i.e. claims apply to the provider only.

Suggested change

// fields are the fields owned by the owner of the claim. If the owner sets values of those

// fields, they will be synced to the other participant.

// Mutually exclusive with preservings.

// fields are a list of JSON Paths describing which parts of an object the provider wants to control in case

// the object is owned by the consumer as a whole. This field cannot be set if provider owned objects

// are selected in this claim, and must be non-empty otherwise.

The owner term was taken from the selector. It describes the binding party that, if not overridden, will be the side to create and delete the resource.
My interpretation of "fields" and "preserving" was that:
"fields" is a whitelist for synchronization, meaning everything in "fields" uses the owner of the claimed resource (owner can be consumer or provider) as the source of truth. Everything not in "fields" uses the other side (if the owner is consumer it used provider) as the source of truth.

"excludes" is a blacklist, meaning that fields that are referenced in "excludes" do not use the owner of the resource as the source of truth, and everything not in "excludes" uses the owner as the source of truth.

These claims are always from the point of view of the provider. There are two cases:

the provider is owner => preserving defines exceptions that the provider (here the owner) does not claim to own

the consumer is the owner => fields defines exceptions that the provider claims to own despite the consumer being the owner.

In other words: the subject of the claim is always the provider. There is no inversion of the subject when the owner changes.

I think there are two points to be clarified here.

Inversion of subject. I understand that we don't want to invert it.

The blacklist/whitelist part.
quoting parts of an example prompt:

read spec.nodeName and update status

How would this be expressed?

sttts · 2023-03-09T15:37:05Z

+	// Preservings are the fields that are preserved by the konnector during synchronization.
+	// The owner is not able to set those fields. If the owner changes the value of these fields,
+	// their change will be overwritten.


Suggested change

// Preservings are the fields that are preserved by the konnector during synchronization.

// The owner is not able to set those fields. If the owner changes the value of these fields,

// their change will be overwritten.

// preserving is a list of JSON Paths describing fields that should be preserved while updating with the

// object state on the provider side. This field cannot be set if consumer owned objects are selected in

// this claim.

sttts · 2023-03-09T15:38:57Z

+	Owner Owner `json:"owner"`
+
+	//
+	// WARNING: If adding new fields, add them to the XValidation check!


does this apply?

Probably not. I would argue it makes sense to make the whole permissionClaim immutable though.
If we make it mutable, and simply take over the changes to the consumer, the provider could change the permission after it has been accepted, leading to a compromised consumer.

sttts · 2023-03-09T15:39:07Z

+type Owner string
+
+const Provider Owner = "Provider"
+const Consumer Owner = "Consumer"


sttts · 2023-03-09T15:39:25Z

+	// +listType=map
+	// +listMapKey=group
+	// +listMapKey=resource
+	// +kubebuilder:validation:MaxItems=2


why only 2?

sttts · 2023-03-09T15:40:06Z

+	// +optional
+	// +listType=map
+	// +listMapKey=group
+	// +listMapKey=resource


don't think we are able to define a key for this list. This is rather a set.

sttts · 2023-03-09T15:40:18Z

 	// a sensible default is chosen by the service provider.
-	Versions []string `json:"versions,omitempty"`
+	Versions         []string          `json:"versions,omitempty"`
+	PermissionClaims []PermissionClaim `json:"permissionClaims,omitempty"`


sttts · 2023-03-09T15:42:30Z

@@ -0,0 +1,69 @@
+/*
+Copyright 2022 The Kube Bind Authors.


sttts · 2023-03-09T15:44:54Z

+// +kubebuilder:subresource:status
+// +kubebuilder:printcolumn:name="Established",type="string",JSONPath=`.status.conditions[?(@.type=="Established")].status`,priority=5
+// +kubebuilder:printcolumn:name="Age",type="date",JSONPath=`.metadata.creationTimestamp`,priority=0
+type APIServiceExportTemplate struct {


this is interesting, especially the selector. Don't think we have explored this side yet. Worth a discussion in the gdoc? My gut feeling is that this is really part of a demo backend, but could have different shape depending on context. For example the claims could differ depending on group membership or on input the UI.

There is a section on the gdoc called "Associating permissionClaims with Resources" which touches on this. I agree with the idea of making this part of the demo backend. Should I move the API to the example backend?

i am actually not understanding why this is needed.

Suggestion: a different API group example-backend.kube-bind.io/v1alpha1

lhaendler · 2023-03-14T13:00:40Z

                  type: object
-                  x-kubernetes-validations:
-                  - message: donate and adopt are mutually exclusive
-                    rule: '!(self.create.donate && self.adopt)'


This validation broke the e2e tests, so i temporarily disabled it.

s-urbaniak · 2023-03-15T12:53:50Z


+type OnConflictOptions struct {
+	// providerOverrides will make the provider override any object that might already exist
+	// in the consumer cluster if it has the same namespaced name as a resource created by the


why the restriction to namespaced name ?

Add new type APIServiceExportTemplate which provides a template for new APIServiceExports the type associates the claimed resources with the CRD Add claimed resource information to APIBinding and APIExport Current Limitation: Validation is commented out because the e2e testing infrastructure rejected the validation for being too computationally expensive

CRDs to be displayed are now based on APIServiceExportTemplates and not on a label Claimed resources are added to the APIServiceExport Claimed resources are sent to the cli plugin

Add prompt for permission claim requests Add permissionclaims including accepted state to APIBinding

Add a new controller and reconciler for downsyncing claimed resources

sttts · 2023-10-16T08:50:11Z

+
+// CreateFromFS creates the given CRDs using the target client from the
+// provided filesystem and waits for it to become established. This call is blocking.
+func CreateFromFS(ctx context.Context, client apiextensionsv1client.CustomResourceDefinitionInterface, fs embed.FS, grs ...metav1.GroupResource) error {


remind me where this code comes from and why we need it? Is it from kcp? Looks familar.

I took it from https://github.com/kube-bind/kube-bind/blob/main/deploy/crd/bootstrap.go

sttts · 2023-10-16T08:52:50Z

+
+const (
+	// GroupName is the group name used in this package
+	GroupName = "example.com"


example-backend.kube-bind.io would be better.

sttts · 2023-10-16T08:55:06Z

 			deleteServiceExportRequest: func(ctx context.Context, ns, name string) error {
 				return bindClient.KubeBindV1alpha1().APIServiceExportRequests(ns).Delete(ctx, name, metav1.DeleteOptions{})
 			},
+			crds: exporttemplate.NewCatalogue(config),


code follows American spelling, i.e. catalog

sttts · 2023-10-16T08:57:36Z

+	templates "github.com/kube-bind/kube-bind/contrib/example-backend/client/clientset/versioned"
+)
+
+type Index struct {


not sure I can follow this construct. What is it?

The backend needs to know which CRD get which claimedresources. I stored them in APIServiceExportTemplates. Index lets the backend get all exported CRDs (those that are referenced by a APIServiceExportTemplate), and to look up the template for a CRD so it can attach the claimed resources

sttts · 2023-10-16T09:31:31Z

+	OnConflict *OnConflictOptions `json:"onConflict,omitempty"`
+
+	// update lists which updates to objects on the consumer cluster are claimed.
+	// By default, the whole object is synced, but metadata is not.


Suggested change

// By default, the whole object is synced, but metadata is not.

// By default, the whole object is continously synced, but metadata is not after creation.

Is this correct?

What would be the reason for syncing metadata on creation, but not afterwards? IIRC we wanted to not sync metadata by default including creation

sttts · 2023-10-16T09:42:36Z

+	// Even if the consumer mistakenly or intentionally
+	// deletes the object, the provider will recreate it. If the field is set as false,
+	// the provider will not recreate the object in case the object is deleted on the consumer cluster.


These are redundant? Might have been old text in our gdoc. Or is there a detail I am missing?

yes, let's remove them from both places

sttts · 2023-10-16T11:08:53Z

+type UpdateOptions struct {
+	// fields are a list of JSON Paths describing which parts of an object the provider wants to control.
+	//
+	// This field is ignored if the owner in the claim selector is set to "Provider".


this wording is strange. The selector selects objects, either consumer or provider owned. This here is about the object owner, not about the selector.

sttts · 2023-10-16T12:16:57Z

+	// Namespaces matches against the metadata.namespace field. A value of "*" matches namespaced objects across all namespaces.
+	// If a resources namespace matches any value in namespaces, the resource namespace is considered matching.
+	// If the claim is for a cluster-scoped resource, namespaces has to explicitly be set to an empty array to prevent defaulting to "*".
+	// If the "names" field is unset, all objects of the group/resource within the listed namespaces (or cluster) will be claimed.


names cannot be unset. It's defaulted to *.

What should we do for cluster-scoped objects?

sttts · 2023-10-16T12:17:37Z

+	// +optional
+	Namespaces []string `json:"namespaces,omitempty"`
+
+	// labelSelectors is a list of label selectors matching selected resources. label selectors follow the same rules as kubernetes label selectors,


A disjunction I assume?

Disjunction for items of the array, conjunction for key-value pairs in the map

sttts · 2023-10-16T12:23:22Z

+		// TODO
+		if onlyFirstServingVersion && !crdVersion.Storage {
+			continue
+		}


would expect this to be a parameter to CRDToServiceExport and controlled through higher level means.

this change can be removed. The intentions were the same as in #181

sttts · 2023-10-16T12:24:19Z

@@ -1,5 +1,5 @@
 /*
-Copyright 2022 The Kube Bind Authors.
+Copyright 2023 The Kube Bind Authors.


we don't update these for existing files.

sttts · 2023-10-16T12:26:38Z

@@ -0,0 +1,824 @@
+/*


would love to see us merge servicebindings.go and this test, plus the API types first, to get this in step by step instead of 6000 lines at once.

sttts · 2023-10-16T12:28:49Z

 }

+func (opts *BindOptions) Cleanup() {
+	os.Remove(opts.outFile.Name())


this is strange to be attached to options. Options are supposed to be just a data structure of flag values.

Rename api group for backend configuration Custom Resources from example.com to example-backend.kube-bind.io

Co-authored-by: Dr. Stefan Schimanski <stefan.schimanski@gmail.com>

sttts · 2023-10-23T15:19:37Z

+	kubebindv1alpha1 "github.com/kube-bind/kube-bind/pkg/apis/kubebind/v1alpha1"
+)
+
+const annotation = "kube-bind.io/resource-owner"


kube-bind.io/owner would be better

A missing nil check for the Selector led to controller crashes

mjudeikis · 2025-10-08T08:37:14Z

implemented as part of #304

lhaendler force-pushed the claimed_resources branch from c2f3eef to 12dcb1a Compare March 9, 2023 12:04

sttts reviewed Mar 9, 2023

View reviewed changes

lhaendler commented Mar 14, 2023

View reviewed changes

s-urbaniak reviewed Mar 15, 2023

View reviewed changes

lhaendler added 7 commits June 6, 2023 11:32

Integrate APIServiceExportTemplate in example backend

cc89536

CRDs to be displayed are now based on APIServiceExportTemplates and not on a label Claimed resources are added to the APIServiceExport Claimed resources are sent to the cli plugin

Add claimed resources to cli

49cf425

Add prompt for permission claim requests Add permissionclaims including accepted state to APIBinding

Add reconciler for Downsync

a14e889

Add a new controller and reconciler for downsyncing claimed resources

Use tempfile instead of stdin to pass manifest to bind apiservice

bd5ccf1

Change API definitions to community proposal

4131eac

Add copyright header

fbbe7b9

Update api definititions

498f595

lhaendler marked this pull request as ready for review August 16, 2023 15:26

Merge branch 'kube-bind:main' into claimed_resources

1297ccd

sttts reviewed Oct 16, 2023

View reviewed changes

Comment thread pkg/apis/kubebind/v1alpha1/apiservicebinding_types.go

sttts reviewed Oct 16, 2023

View reviewed changes

Comment thread pkg/apis/kubebind/v1alpha1/apiservicebinding_types.go Outdated

sttts reviewed Oct 16, 2023

View reviewed changes

lhaendler and others added 4 commits October 18, 2023 14:47

Rename example-backend api group

da72fcc

Rename api group for backend configuration Custom Resources from example.com to example-backend.kube-bind.io

Rename function to american spelling

d2e061d

Update pkg/apis/kubebind/v1alpha1/apiservicebinding_types.go

3477587

Co-authored-by: Dr. Stefan Schimanski <stefan.schimanski@gmail.com>

Update pkg/apis/kubebind/v1alpha1/apiservicebinding_types.go

0d12467

Co-authored-by: Dr. Stefan Schimanski <stefan.schimanski@gmail.com>

lhaendler force-pushed the claimed_resources branch from 84ddc19 to 0d12467 Compare October 18, 2023 13:34

Remove redundant paragraph

b8618b1

lhaendler mentioned this pull request Oct 18, 2023

Claimed resources api #185

Closed

fixup! Add fields for claimed resources

b317fda

sttts reviewed Oct 23, 2023

View reviewed changes

Fix: nil check in claimed_resources_controller

1814fb5

A missing nil check for the Selector led to controller crashes

mjudeikis closed this Oct 8, 2025

	// PermissionClaim identifies an object by GR and identity hash.
	// PermissionClaim selects objects of a GVR that the service provider ...

	// By default, the whole object is synced, but metadata is not.
	// By default, the whole object is continously synced, but metadata is not after creation.

Uh oh!

Conversation

lhaendler commented Mar 9, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Scope

Current limitations

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sttts Mar 9, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

lhaendler commented Mar 9, 2023 •

edited

Loading

sttts Mar 9, 2023 •

edited

Loading